Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-14973 Add PII check to Socure flow #11747

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jmax-gsa
Copy link
Contributor

🎫 Ticket

LG-14973

🛠 Summary of changes

Add the PII checks in the DocPiiForm to the Socure flow.

📜 Testing Plan

Verify that the tests in in spec/services/doc_auth/socure/responses/docv_result_response_spec.rb adequately check the functionality and pass.

@jmax-gsa jmax-gsa requested review from amirbey and a team January 14, 2025 15:55
Copy link
Contributor

@theabrad theabrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@amirbey amirbey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we decided to defer displaying PII errors in LG-15307, here when PII fails the below is displayed to the user.
Screenshot 2025-01-15 at 12 27 06 PM

the failing PII scenario can recreated using http://localhost:3000/verify/socure/document_capture_update?docv_token=1111-1111 which passes back an expired date

i think adding a test would be helpful to make sure the user is seeing what we're intending ... thanks!

@jmax-gsa jmax-gsa force-pushed the jmax/LG-14973-add-pii-check-to-socure-flow branch from d9d6b9e to 8568002 Compare January 16, 2025 19:04
if !successful_result?
{ socure: { reason_codes: get_data(DATA_PATHS[:reason_codes]) } }
elsif !pii_valid?
{ pii_validation: 'failed' }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could call this like, local_attribute_validation or something and not have to worry about having pii in the key name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda prefer the clarity, since we have a ticket coming up shortly to actually display more specific errors about pii, and would likely want to use pii* keys anyway; might as well cope with it now.

@jmax-gsa jmax-gsa force-pushed the jmax/LG-14973-add-pii-check-to-socure-flow branch from 8568002 to 8c68295 Compare January 16, 2025 20:38
Check PII using DocPiiForm.

changelog: Upcoming Features,Socure,Add Idv::DocPiiForm check to Socure flow.
@jmax-gsa jmax-gsa force-pushed the jmax/LG-14973-add-pii-check-to-socure-flow branch from 8c68295 to 422ed47 Compare January 16, 2025 20:40
@jmax-gsa
Copy link
Contributor Author

Merging following conversation with @amirbey. Further work from that conversation to be done in LG-15309.

@jmax-gsa jmax-gsa requested a review from amirbey January 17, 2025 17:49
Copy link
Contributor

@amirbey amirbey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@amirbey amirbey self-requested a review January 17, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants